Skip to content

MG-2059 - Set correct log level depending on error #2196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 20 commits into
base: main
Choose a base branch
from
Draft

MG-2059 - Set correct log level depending on error #2196

wants to merge 20 commits into from

Conversation

@nyagamunene
Copy link
Contributor

@nyagamunene nyagamunene commented Apr 23, 2024

What type of PR is this?

This is a refactor because it sets correct log level depending on error.

What does this do?

It fixes the correct log level for the logs. Currently all log are warnings.

Which issue(s) does this PR fix/relate to?

Have you included tests for your changes?

No.

Did you document any new/modified feature?

No.

Notes

@nyagamunene nyagamunene force-pushed the MG2059-ErrorRefactoring branch 2 times, most recently from 533c151 to a596fd9 Compare April 29, 2024 07:23
@nyagamunene nyagamunene marked this pull request as ready for review April 29, 2024 07:23
arvindh123
arvindh123 requested changes Apr 30, 2024
View reviewed changes
Copy link
Contributor

@arvindh123 arvindh123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the following files, I found some error are logged as Warn :

auth/api/logging.go L30 to L128 https://github.com/nyagamunene/magistrala/blob/a596fd9abbf62be732b9185855def1f492f3baeb/auth/api/logging.go#L37-L129

bootstrap/api/logging.go

bootstrap/postgres/configs.go

certs/api/logging.go

cmd/opcua/main.go

coap/api/transport.go

consumers/notifiers/api/logging.go

consumers/writers/api/logging.go

internal/groups/api/logging.go

consumers/writers/api/logging.go

internal/groups/api/logging.go

lora/api/logging.go

invitations/middleware/logging.go

opcua/api/logging.go

pkg/events/redis/subscriber.go

things/api/logging.go

twins/api/logging.go

users/api/logging.go

ws/api/endpoints.go

rodneyosodo
rodneyosodo requested changes May 3, 2024
View reviewed changes
Copy link
Contributor

@rodneyosodo rodneyosodo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some errors should be logged with Warn while others should be logged with Error. This is a simple explanation of how to do it https://stackoverflow.com/a/2031195 . Also, consider using ErrorContext and pass in the context rather than Error

@nyagamunene nyagamunene force-pushed the MG2059-ErrorRefactoring branch 2 times, most recently from 95f66c4 to 7022dd0 Compare May 9, 2024 08:41
@dborovcanin dborovcanin force-pushed the MG2059-ErrorRefactoring branch from 7022dd0 to a8be062 Compare May 15, 2024 12:28
rodneyosodo
rodneyosodo requested changes May 17, 2024
View reviewed changes
auth/api/logging.go Outdated
if err != nil {
args = append(args, slog.Any("error", err))
lm.logger.Warn("List objects failed to complete successfully", args...)
lm.logger.ErrorContext(ctx, "List objects failed to complete successfully", args...)
Copy link
Contributor

@rodneyosodo rodneyosodo May 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert all errors to WarnContext

ws/api/endpoints.go Outdated
conn, err := upgrader.Upgrade(w, r, nil)
if err != nil {
logger.Warn(fmt.Sprintf("Failed to upgrade connection to websocket: %s", err.Error()))
logger.Error(fmt.Sprintf("Failed to upgrade connection to websocket: %s", err.Error()))
Copy link
Contributor

@rodneyosodo rodneyosodo May 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use parent context

ws/api/endpoints.go Outdated
channelParts := channelPartRegExp.FindStringSubmatch(r.RequestURI)
if len(channelParts) < 2 {
logger.Warn("Empty channel id or malformed url")
logger.Error("Empty channel id or malformed url")
Copy link
Contributor

@rodneyosodo rodneyosodo May 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use request context

@arvindh123 arvindh123 force-pushed the MG2059-ErrorRefactoring branch from a8be062 to bdcbd75 Compare May 20, 2024 13:27
@nyagamunene nyagamunene force-pushed the MG2059-ErrorRefactoring branch from bdcbd75 to 567b80a Compare May 20, 2024 22:26
@nyagamunene nyagamunene changed the title MG-2059-Set correct log level depending on error MG-2059 - Set correct log level depending on error May 21, 2024
@nyagamunene nyagamunene self-assigned this Jun 6, 2024
@nyagamunene nyagamunene marked this pull request as draft June 6, 2024 10:36
@nyagamunene nyagamunene force-pushed the MG2059-ErrorRefactoring branch from 8fb868f to ac0a377 Compare June 12, 2024 17:21
nyagamunene added 15 commits June 12, 2024 20:31
@nyagamunene
Reforctored errors in auth
d9827d4 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Reforctored errors in bootstrap
a36e033 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Reforctored errors in certs
afe9fc7 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Reforctored errors in coap
221bffc 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Error handling refactoring
a4e75fc 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Removed warn on remaining logs
218c4fa 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Changed from error to errorcontext
fbce207 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Changed from error to errorcontext
322eb95 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Changed from error to errorcontext
b7a7346 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Changed from error to errorcontext in redis handle
78c260d 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Used parent context
895d279 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
used request context
cc76620 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Reverted to warn context
20d7856 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Refactor: Pkg Error Package
f1ef040 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Refactor: Users and things
30d2694 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Refactor: changed the errors.Contains function
13d7c6f 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Refactor: Error types
9cd9e43 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
Refactor: Reverted back to original errors.Contains
363c11d 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene nyagamunene force-pushed the MG2059-ErrorRefactoring branch from 7911b5b to 363c11d Compare June 12, 2024 17:35
@nyagamunene
Added rollback error
7aacba8 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene
linted code
bf977ec 
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@arvindh123 arvindh123 requested a review from felixgateru July 16, 2025 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rodneyosodo rodneyosodo rodneyosodo requested changes

@arvindh123 arvindh123 arvindh123 requested changes

@felixgateru felixgateru Awaiting requested review from felixgateru

Requested changes must be addressed to merge this pull request.

Assignees

@nyagamunene nyagamunene

Labels

enhancement help-wanted later

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Set correct log level depending on error

4 participants

@nyagamunene @rodneyosodo @arvindh123 @dborovcanin